pipewire: context hardening — combines #226 + #227, reconciled#229
Conversation
|
Follow-up commit 91ecbd5: Version archaeology (verified per tag in the pipewire sources,
No blocking invoke is correct against all three states — so the new Validation on this branch: ossia score's 28-cell PipeWire round-trip matrix (24 PASS + 4 honest-fallback, zero |
|
@pri-yan-shu do you think you could test this last pipewire PR ? As it involves the pipewire work which had deadlock / crash issues depending on the linux distro and pipewire version |
91ecbd5 to
86d7e1d
Compare
|
@ogauthiersat I refactored and moved the PW tests you added with the other tests, if you can also test again :) |
70682d0 to
ae628c7
Compare
|
I'm on pipewire 1.6.7, all tests pass. Anything else I can test? |
|
thanks @pri-yan-shu ! |
Fix thread-loop lock and iteration bugs in the shared PipeWire context, surfaced by concurrent audio + MIDI use and by video round-trip testing: - synchronize(): a foreign-thread sync no longer iterates the pw_loop itself (two threads driving one loop corrupt its per-source dispatch state and crash in the core-socket callback) nor holds pipewire's recursive lock across the wait. It issues pw_core_sync under a brief lock and waits on a private condition variable that the core done/error callbacks notify. - reconnect(): runs on the calling thread instead of via invoke_sync on the worker. tear_down() destroys the very loop the worker runs, so reconnecting from it self-destructs the loop and hangs. - invoke_sync(): queues a non-blocking pw_loop_invoke and waits on its own condition variable, bounded by sync_deadline. The blocking-invoke lock contract is unstable across PipeWire versions; the non-blocking path is correct on every release. - format negotiation advertises buffer size/stride as ranges so producers with padded rows (GPU-allocated strides) can link. Co-Authored-By: Olivier Gauthier <ogauthier@sat.qc.ca> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Three standalone programs under tests/integration/ exercising the thread-loop context fixes: sync (create/synchronize/destroy churn and a sustained synchronize loop), reconnect (repeated reconnect from the calling thread), and subscriptions (subscribe/unsubscribe churn, filter teardown with live subscriptions, and synchronize from a loop-thread callback). Each skips with exit 0 when no PipeWire daemon is reachable and arms a watchdog so a lock-corruption regression fails loudly instead of hanging. Co-Authored-By: Olivier Gauthier <ogauthier@sat.qc.ca> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ae628c7 to
75a3d27
Compare
|
@jcelerier All tests passes on trixie with pipewire 1.4.2. I also test with score, it resolves the crash I had in pipewire client at process exit. I could successfully stream a midi file from Ardour. I tested restarting audio engine while having a midi device connected and it appears to behave correctly. |
|
\o/ thanks all ! |
Combines #226 (@ogauthiersat) and #227 into one coherent series, with the two
invoke_syncfixes reconciled — merging both PRs as-is would auto-merge cleanly and deadlock (each adds its own lock around the blocking invoke; the recursive mutex would be held twice, and the loop-control hooks release exactly one level, so the worker could never dispatch the invoke).Contents:
invoke_synclock+guard, thesynchronize()rewrite (no more two threads iterating onepw_loop—pw_core_sync+ private CV notified fromon_core_done), thereconnect()self-destruct fix (no morepthread_join(self)via invoke_sync), and the three watchdog-armed regression examples.format.hppchange kept:SPA_PARAM_BUFFERS_size/strideasCHOICE_RANGE(tight, tight, INT32_MAX)so padded-stride producers (GPU allocations) can link.with_lockwrappers dropped in favor of pipewire: fix thread-loop lifetime & recursive-lock crashes in the shared context #226's single guard (same semantics — lock held exactly once at the invoke, per the hook contract inthread-loop.cimpl_before/impl_after); plus a defensivem_thread_loopnull check in the early return so a mid-teardowninvoke_synccan't lock a null loop.Validation: ossia score's
PipewireRoundtripharness against a live PipeWire 1.3.0 daemon — 28-cell format×transport matrix (SHM + DMA-BUF, OpenGL + Vulkan, 12 pixel formats incl. padded strides): 24 PASS + 4 PASS(honest-fallback), zero deadlocks/asserts across repeated stream create/destroy cycles. #226's regression examples compile and are included unchanged.Supersedes #227 (I'll close it); #226 can be closed in favor of this or merged first — this branch contains it verbatim either way.
🤖 Generated with Claude Code
https://claude.ai/code/session_014rZgzE8JjWvHDtaVUhxpLE
Update (91ecbd5):
invoke_syncis now rewritten to a non-blockingpw_loop_invoke+ private mutex/CV with async_deadlinebound, instead of a lock-held blocking invoke. Rationale: the blocking-invoke lock contract flip-flopped across PipeWire releases (hooks removed/reverted three times upstream; 1.3.0–1.3.80 and 1.5.0–1.5.80 dev clients deadlock with a held lock, all 1.2/1.4 releases require it, 1.5.81+ accepts either) — the non-blocking pattern is contract-stable on every series. Details and the per-series matrix in the comments below. Validated with score's 28-cell round-trip harness and the three regression examples from #226.